Skip to content

Conversation

@taylorwilsdon
Copy link
Contributor

@taylorwilsdon taylorwilsdon commented Jun 1, 2025

Detailed Feature Request: #4218

Implement MCP Streamable HTTP Support in same manner as SSE & stdio

The existing pinned version of the client SDK already supports Streamable HTTP transport—this PR simply exposes that capability in our extension alongside the existing stdio and SSE options.

I've been watching several efforts related to implementing Streamable HTTP but they seem to be tied to enormous refactors that fundamentally change the Roo backend and haven't been updated in nearly a month, so I figured others might benefit from my local branch, which cleanly adds streamable HTTP support in the same manner as stdio and SSE are implemented exactly.

Related GitHub Issue

Opened an issue at #4216
Will close that, #3627 and #2131

Description

This pull request introduces support for a new Model Context Protocol (MCP) server type: streamable-http. This allows Roo Code to connect to MCP servers that use a streamable HTTP-based transport, expanding its integration capabilities.

Key changes include:

  • Added Streamable HTTP Transport to McpHub.ts (src/services/mcp/McpHub.ts):
    • Imported and integrated StreamableHTTPClientTransport from the MCP SDK.
    • Updated the McpConnection type to include StreamableHTTPClientTransport.
    • Enhanced server configuration validation (validateServerConfig) to:
      • Recognize the streamable-http type.
      • Provide specific error messages for streamable-http configurations (e.g., requiring a url).
      • Ensure that configurations with a url explicitly define their type as either sse or streamable-http.
    • Modified the connectServer method to:
      • Handle the instantiation and setup of StreamableHTTPClientTransport for streamable-http type servers.
      • Include appropriate onerror and onclose handlers for this new transport type, similar to the existing SSEClientTransport.
  • New Transport Mock: Added src/__mocks__/@modelcontextprotocol/sdk/client/streamable-http.js to mock the StreamableHTTPClientTransport for testing purposes.

Test Procedure

  1. Unit Tests:
    • Review existing unit tests for McpHub.ts to ensure they cover the new validation logic for streamable-http configurations.
    • If necessary, add new test cases specifically for streamable-http connection attempts, error handling, and successful connections using the new mock.
  2. Manual Testing:
    • Add a new MCP server configuration to your settings.json with type: "streamable-http" and a valid (or mock) url.
      "mcp.servers": {
        "myStreamableServer": {
          "type": "streamable-http",
          "url": "http://localhost:8080/stream", // Replace with your test endpoint
          "displayName": "My Streamable HTTP Server"
        }
      }
    • Start Roo Code and verify:
      • The server attempts to connect.
      • The status of the server (connecting, connected, disconnected, error) is correctly displayed in the MCP panel.
      • If using a mock server, verify that tool calls or resource access (if implemented by the mock) function as expected.
    • Test error scenarios:
      • Invalid URL.
      • Server not reachable.
      • Server disconnects after connection.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Showing it running with Google Workspace MCP via Streamable HTTP
image

Documentation Updates

There is seemingly no references to MCP setup present in the README.md so I did not include that in the scope of this PR. Happy to add it myself, will follow with a second separate PR that covers stdio, sse & streamable-http. User-facing documentation regarding MCP server configuration will need to be updated to include the new streamable-http type, its purpose, and its specific configuration options (url, optional headers).

Additional Notes

This change aligns the MCP client capabilities more closely with the available transport mechanisms in the @modelcontextprotocol/sdk.

Get in Touch

taylorwilsdon


Important

Adds streamable-http transport support for MCP servers in McpHub.ts, including validation and connection handling.

  • Behavior:
    • Adds streamable-http support in McpHub.ts for MCP servers, allowing connections via streamable HTTP transport.
    • Updates validateServerConfig to recognize streamable-http type and require url.
    • Modifies connectToServer to handle StreamableHTTPClientTransport instantiation and error handling.
  • Mocks:
    • Adds streamable-http.js mock for StreamableHTTPClientTransport in src/__mocks__ for testing.

This description was created by Ellipsis for 1fc6d52. You can customize this summary. It will automatically update as commits are pushed.

@taylorwilsdon taylorwilsdon requested review from cte and mrubens as code owners June 1, 2025 20:05
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jun 1, 2025
@taylorwilsdon
Copy link
Contributor Author

Showing it successfully running (redacted my emails/PII):
image

@taylorwilsdon
Copy link
Contributor Author

taylorwilsdon commented Jun 1, 2025

Hey @hannesrudolph - chatted with you a handful of times on reddit but this is my first upstream roo PR, let me know if I missed anything from the contribution guide. I read through CONTRIBUTING.md and shot you a ping on discord. Thanks!

btw the failing test (user_feedback_diff) also appears present in the main branch without any changes and isn't related to my change, but happy to fix while I was under the hood. If you don't want that part in here, let me know and I can drop from the change and separate out.

@taylorwilsdon
Copy link
Contributor Author

taylorwilsdon commented Jun 1, 2025

Fixed unrelated failing tests with this branch @mrubens mentioned

roo-cline:test: ...........*....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................*...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
roo-cline:test: Ran 2074 tests in 25.731 s
roo-cline:test:  2053 passing 0 failing 21 pending
roo-cline:test: 
roo-cline:test:  RUN  v3.1.3 /Users/taylorwilsdon/git/Roo-Code/src
roo-cline:test: 
roo-cline:test:  ✓ integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts (7 tests) 5ms
roo-cline:test:  ✓ integrations/terminal/__tests__/ExecaTerminal.spec.ts (1 test) 13ms
roo-cline:test:  ✓ api/providers/__tests__/ollama.spec.ts (8 tests) 4ms
roo-cline:test:  ✓ api/providers/__tests__/openai-usage-tracking.spec.ts (3 tests) 3ms
roo-cline:test:  ✓ api/providers/__tests__/openai-native.spec.ts (21 tests) 6ms
roo-cline:test:  ✓ api/providers/__tests__/openai.spec.ts (20 tests) 7ms
roo-cline:test:  ✓ api/providers/fetchers/__tests__/openrouter.spec.ts (2 tests) 32ms
roo-cline:test: 
roo-cline:test:  Test Files  7 passed (7)
roo-cline:test:       Tests  62 passed (62)
roo-cline:test:    Start at  16:36:36
roo-cline:test:    Duration  396ms (transform 389ms, setup 0ms, collect 1.40s, tests 69ms, environment 1ms, prepare 264ms)
roo-cline:test: 
roo-cline:test: 

Tasks:    7 successful, 7 total
Cached:    6 cached, 7 total
Time:    28.423s 

@mrubens
Copy link
Collaborator

mrubens commented Jun 2, 2025

Thanks for this! Will take a look ASAP. I rolled back the PR that broke tests but I can look at applying your fix there as a follow up.

@taylorwilsdon
Copy link
Contributor Author

Thanks for this! Will take a look ASAP. I rolled back the PR that broke tests but I can look at applying your fix there as a follow up.

Awesome, appreciate it! The test fix changes can be pruned from the branch cleanly just by dropping that one file so can totally do that but fwiw fix looked good if useful 👍

// Streamable HTTP connection
transport = new StreamableHTTPClientTransport(new URL(configInjected.url), {
requestInit: {
headers: configInjected.headers,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this hack in #4148 to make headers work with SSE - do you know if it's also needed for streamable? Hopefully not 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm good call, may not be necessary but wanted it match the existing patterns

Looking at it, EventSource can’t natively send arbitrary request headers from the browser, so #4148 injects them through the polyfill’s custom fetch hook. 

StreamableHTTPClientTransport opens its connection with fetch() under the hood. Because it relies on plain fetch, anything you put in options.requestInit.headers is forwarded automatically; no special shim is required.

The only gotcha is to pass the options object as the second (not third) argument to the transport constructor. Don’t think it poses any usability issues as is, didn’t in my testing but want to make everything clean and tidy can poke at it in the am

Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you! One question about the headers but even if we need something additional we can do it as a follow-up.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 2, 2025
@mrubens mrubens merged commit 7e125f8 into RooCodeInc:main Jun 2, 2025
11 checks passed
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jun 2, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants